Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amateur golden point #76

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

Jorman
Copy link

@Jorman Jorman commented Oct 23, 2023

I update file by file manually, so I hope I modify all the files, let me know if is working, on my side is working but because I have all the file of course

closes #69

source/model/matchConfig.mc Outdated Show resolved Hide resolved
source/model/matchStatus.mc Outdated Show resolved Hide resolved
source/model/matchStatus.mc Outdated Show resolved Hide resolved
source/model/PadelMatch.mc Outdated Show resolved Hide resolved
Removed commented line
Renamed TotalDeuces -> totalDeuces
resources/menus/number_of_deuces.xml Outdated Show resolved Hide resolved
source/model/PadelMatch.mc Show resolved Hide resolved
source/model/PadelMatch.mc Outdated Show resolved Hide resolved
I forgot one if...else section in incP1 function
Added the Golden status to see when the golden happen, so both scores are G - G
Update to have Golden status available
Copy link
Author

@Jorman Jorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a visual way to know when there's a Golded point, I know that maybe all will change, but you know, better to have, in this way is not possible to forget

Copy link
Owner

@pedrorijo91 pedrorijo91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finally looking again at your PR, sorry for taking this huge amount of time!

i merged the current master branch into your PR, and also added unit tests to help you. you can see the changed I added here https://github.com/pedrorijo91/garmin-padel/pull/76/commits

We just need to sort the thing with the scoreIdx whenever you have some time please

@@ -57,6 +61,10 @@ class MatchStatus {
self.p1ScoreIdx++;
}

function getP1Idx() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is somehow new. the p1Idx and p2Idx were not exposed on purpose. it's an internal data structure that should not be used by anyone else other than the matchStatus class. callers should look into the score only

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this change happened because we want to display the G-G score when in golden point right? i feel we should revert it. instead of doing this, can we infer we are in the G-G state by looking at the score and at the number of deuces?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is somehow new. the p1Idx and p2Idx were not exposed on purpose. it's an internal data structure that should not be used by anyone else other than the matchStatus class. callers should look into the score only

How come not here? That's the file where I found all the functions, it seemed more practical to draw from here since you have an index with all the scores.
So if I understand you correctly, you would rather create a second function that does the calculations and just returns the value? How were you planning to proceed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this change happened because we want to display the G-G score when in golden point right? i feel we should revert it. instead of doing this, can we infer we are in the G-G state by looking at the score and at the number of deuces?

Trying this feature, I realized that a visual indication is better, because it happens that the point lasts a very long time, so long that you don't remember accurately the score, which is then one of the purposes of the application. I think it's happened to you playing as well, points of a long duration that at the conclusion you don't know the score. So the ploy, you run out of deuces available, the application points to G-G which reminds you that you are at the Golden Point and the point is decisive. The same thing happens with A if you think about it.
I would take advantage of the dedicated array of scores and "play" by moving the index time by time.
How would you do that?

@Jorman
Copy link
Author

Jorman commented May 21, 2024

How do you use the tests you put in? I usually to do the tests I would run the debug and test the functions, without doing anything else.

@pedrorijo91
Copy link
Owner

i just use vscode for running tests. i have a "tests" tab in the left side and then click run:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] Golden Point after X deuce
2 participants